fix(matcher): add workaround hint to invalid params warning by shanliuling · Pull Request #2662 · vuejs/router

shanliuling · 2026-03-16T12:24:38Z

What is the problem?

When using a catch-all route with a named redirect like:

{
  path: '/:pathMatch(.*)*',
  redirect: { name: 'HOME' }
}

Users see a warning:
Discarded invalid param(s) "pathMatch" when navigating.

The warning message doesn't explain how to fix the issue.

How did I fix it?

Enhanced the warning message to include a helpful workaround hint:
If you are using a catch-all route with a named redirect, pass an empty params object: redirect: { name: '...', params: {} }.

This matches the workaround suggested by @posva in the issue comments.

How was this tested?

Updated the existing test in packages/router/__tests__/warnings.spec.ts to match the new warning format
The test now also checks for the presence of the workaround hint

Closes #1617

Summary by CodeRabbit

Bug Fixes
- Clarified discarded/invalid route-parameter warning: an extra guidance hint is now appended only for catch-all style routes with named redirects when the previous location contains params; other cases retain the original wording and link.
Tests
- Added and updated tests to verify the exact warning text and the conditional presence or absence of the workaround hint.

netlify · 2026-03-16T12:24:45Z

✅ Deploy Preview for vue-router canceled.

Name	Link
🔨 Latest commit	`0f74450`
🔍 Latest deploy log	https://app.netlify.com/projects/vue-router/deploys/69c0a5d0bd29070008a9174a

coderabbitai · 2026-03-16T12:25:08Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 991288ca-43b0-4a1b-9cac-7dc23c824629

📥 Commits

Reviewing files that changed from the base of the PR and between e900bb7 and 0f74450.

📒 Files selected for processing (2)

packages/router/__tests__/warnings.spec.ts
packages/router/src/matcher/index.ts

✅ Files skipped from review due to trivial changes (1)

packages/router/src/matcher/index.ts

🚧 Files skipped from review as they are similar to previous changes (1)

packages/router/tests/warnings.spec.ts

📝 Walkthrough

Walkthrough

Adjusts the dev-only warning emitted when resolving named-route navigation that discards invalid location.params; warning now conditionally appends a catch-all redirect hint when implicit params are inherited from the previous location. Tests updated to assert presence/absence of the hint across scenarios.

Changes

Cohort / File(s)	Summary
Matcher change `packages/router/src/matcher/index.ts`	Modify the development warning for discarded/invalid `location.params` during named-route resolution: if the resolved record is a catch-all (no `keys`) and invalid param keys exist in `currentLocation.params` (implicitly inherited), append a hint to pass an explicit empty `params` object for named redirects (`redirect: { name: '...', params: {} }`).
Tests updated `packages/router/__tests__/warnings.spec.ts`	Updated existing test to assert the workaround hint is NOT emitted when params are explicitly passed in `to`; added tests asserting the discarded-param warning plus the catch-all hint when params are implicitly inherited from `from`, and asserting the hint is suppressed when `from.params` is empty.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibble at warnings beneath the moonlit logs,
A hop, a sniff, I tidy stray param frogs.
"Pass empty params" — a gentle, whispered nudge,
So redirects skip leftovers, free to trudge.
Rabbit twitches whiskers, hops off with a shrug. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding a workaround hint to the invalid params warning in the matcher module.
Linked Issues check	✅ Passed	The PR successfully implements the objective from issue `#1617` by adding a conditional workaround hint to the invalid params warning that guides users to use empty params in named redirects.
Out of Scope Changes check	✅ Passed	All changes are directly related to the linked issue: test updates verify the new warning behavior, and the matcher module enhancement adds the conditional workaround hint as specified.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

pkg-pr-new · 2026-03-18T15:51:58Z

Open in StackBlitz

npm i https://pkg.pr.new/vue-router@2662

commit: e5667ca

codecov · 2026-03-18T15:54:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.66%. Comparing base (47f0334) to head (e5667ca).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2662   +/-   ##
=======================================
  Coverage   85.66%   85.66%           
=======================================
  Files          86       86           
  Lines       10007    10007           
  Branches     2289     2289           
=======================================
  Hits         8572     8572           
  Misses       1422     1422           
  Partials       13       13

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

posva

Thanks but this adds noise for all misses. It should only be displayed if the route is a named redirect with no params and the initial target location contained params

shanliuling · 2026-03-19T15:02:21Z

Hi @posva, I've updated the code to only show the workaround hint when the target route has no params AND the current location has params. This should avoid showing the hint for all redirect failures.

The changes:

Added \hasParamsToSuggest\ check: !matcher!.keys.length && Object.keys(currentLocation.params).length\
Only shows the workaround hint when both conditions are met

Please let me know if this matches your requirements.

posva · 2026-03-19T15:07:52Z

packages/router/__tests__/warnings.spec.ts

-    expect('invalid param(s) "no", "foo" ').toHaveBeenWarned()
+    expect('invalid param(s) "no", "foo"').toHaveBeenWarned()
+    // workaround hint should be shown
+    expect('If you are using a catch-all route').toHaveBeenWarned()


shouldn't this not be shown? It would be nice to also have a dedicated unit test of the warning that is being added

posva · 2026-03-19T17:16:26Z

The test still seems wrong and no new test has been added

shanliuling · 2026-03-20T02:40:37Z

Hey @posva, thanks for the feedback! I've updated the tests to be more specific. Now there's a new test case checking that the hint only shows up when there are actually params from the initial location, just like you suggested. I also split the tests to make sure we're not adding noise when there aren't any params to suggest. Let me know if this looks good to you!

posva

The tests still do not look good: they don't showcase the redirect issue and the warning seems to appear even without it

…rect (vuejs#1617)

posva · 2026-03-22T16:35:22Z

packages/router/src/matcher/index.ts

        if (invalidParams.length) {
+          // Check if this is a catch-all route (contains pathMatch) with a named redirect
+          const isCatchAllWithRedirect =
+            matcher!.record.path.includes('pathMatch') && // catch-all route


It’s not about the shape of the path, you need to check for params being passed implicitly.

shanliuling · 2026-03-23T02:33:13Z

Hi @posva, thanks for pointing out the implicit params logic! You were completely right—checking the shape of the path was fundamentally flawed.

I have completely refactored the approach:

Removed all \pathMatch\ and
edirect\ shape checks.
The warning hint is now only triggered when the discarded parameters were passed implicitly (by verifying if \invalidParams\ exist in \currentLocation.params), and the target route has no params.
Rewrote the tests to correctly showcase this exact implicit-passing scenario without adding noise to explicit misses.

This should be fully aligned with your intended logic. Please take a look!

posva requested changes Mar 18, 2026

View reviewed changes

shanliuling mentioned this pull request Mar 19, 2026

fix(matcher): only show workaround hint when target route has no params #2665

Closed

posva requested changes Mar 19, 2026

View reviewed changes

posva requested changes Mar 20, 2026

View reviewed changes

fix(matcher): show workaround hint only for catch-all with named redi…

e900bb7

…rect (vuejs#1617)

shanliuling force-pushed the fix/catch-all-redirect-warning branch from e68daaf to e900bb7 Compare March 22, 2026 16:30

posva requested changes Mar 22, 2026

View reviewed changes

fix(matcher): detect implicit params for catch-all redirect warning hint

0f74450

Uh oh!

Conversation

shanliuling commented Mar 16, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the problem?

How did I fix it?

How was this tested?

Summary by CodeRabbit

Uh oh!

netlify bot commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for vue-router canceled.

Uh oh!

coderabbitai bot commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

pkg-pr-new bot commented Mar 18, 2026

Uh oh!

codecov bot commented Mar 18, 2026

Codecov Report

Uh oh!

posva left a comment

Choose a reason for hiding this comment

Uh oh!

shanliuling commented Mar 19, 2026

Uh oh!

posva Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

posva commented Mar 19, 2026

Uh oh!

shanliuling commented Mar 20, 2026

Uh oh!

posva left a comment

Choose a reason for hiding this comment

Uh oh!

posva Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

shanliuling commented Mar 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shanliuling commented Mar 16, 2026 •

edited by coderabbitai bot

Loading

netlify bot commented Mar 16, 2026 •

edited

Loading

coderabbitai bot commented Mar 16, 2026 •

edited

Loading